Destroy Dataset with a Mapped DataFile #6860
Conversation
…side, and possible remote layer data. #4093
| // We have the dedicatd command DeleteMapLayerMetadataCommand, but | ||
| // there's no need to use it explicitly, since the Dataverse-side | ||
| // MapLayerMetadata entity will be deleted by the database | ||
| // cascade on the DataFile. -- L.A. Apr. 2020 |
There was a problem hiding this comment.
I had forgotten that there's a DeleteMapLayerMetadataCommand but this comment makes me wonder why we don't call it. I get that we don't need to call it thanks to the cascade, but would calling the command cause problems? Does it do more than we want? It seems to clean up thumbnails, for example. Also, there's tiny typo above: dedicatd.
There was a problem hiding this comment.
I'm still curious about these questions but there's no need to hold this up from QA so I moved it over. This pull request adds a lot of value.
There was a problem hiding this comment.
Sorry, missed the original question 4 hours ago. I guess the answer is, the delete layer command is a more surgical tool; it's for a case where we just need to delete the layer, without touching the file.
Calling it in the context of trying to wipe everything, including the datafile and all its associated physical files, would not hurt, strictly speaking - but I couldn't think of why we would want to call it either; seeing how the cascade is more reliable? It does remove the map thumbnails - but then we run a full physical file cleanup after DeleteDataFileCommand, that will delete all aux files associated with the datafile, thumbnails and otherwise.
And then again there is the fact the command doesn't do everything we need, either. It does not attempt to go out and remove whatever possible data may be sitting on the WorldMap side. I could only assume that it was by design.
My attitude with this PR was to check in something ASAP that works and doesn't break things.
I generally sympathize with the goals of making things cleaner; calling commands instead of database queries and such. But in the case of WorldMap, the whole is a big hack - isn't it? Our attempt to "think about it some more" to see if we were doing everything right resulted in a 2.5 year delay, and a whole bunch of these un-destructible datasets still sitting in production...
There was a problem hiding this comment.
Yes, agreed. I don't think this will break anything and it definitely adds value. Thanks for the explanation.
|
Let's hold off on moving to QA pending the question/typo above |
What this PR does / why we need it:
Self-explanatory; see issue.
Which issue(s) this PR closes:
Closes #4093
Special notes for your reviewer:
It implements cascade delete on both the map layer and tokens; as in the original PR 2.5 yrs ago. In addition, the code is added to the DeleteDataFileCommand, that deletes any data associated with the layer that may exist on the WorldMap side.
Suggestions on how to test this:
Deploy on demo, destroy all those old, as of now un-destroyable datasets.
Does this PR introduce a user interface change?:
no
Is there a release notes update needed for this change?:
no
Additional documentation: